-
-
Notifications
You must be signed in to change notification settings - Fork 680
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: add tests for netlify functions #2493
Conversation
* change url to asyncapi.com * add test script to package.json
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-2493--asyncapi-website.netlify.app/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is so great we are gonna have tests for this finally! 🙌 🙌 🙌 🙌
I left some comments containing suggestions and change requests.
* assert returned content type
@Shurtu-gal In order to clarify the desired coverage of the test, I created the following flowchart explaining the flow the edge function should accomplish. EDIT: please find it here. Based on that, I believe we should have a test for each result of every decision (diamonds). WDYT? |
This pull request has been automatically marked as stale because it has not had recent activity 😴 It will be closed in 120 days if no further activity occurs. To unstale this pull request, add a comment with detailed explanation. There can be many reasons why some specific pull request has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model. Let us figure out together how to push this pull request forward. Connect with us through one of many communication channels we established here. Thank you for your patience ❤️ |
@Shurtu-gal are you still up on this? |
Yeah, sorry this totally went out of my mind. Let me look at it again. Might need some time to get what I was doing though 🤣 |
/ptal |
@derberg @magicmatatjahu @akshatnema @sambhavgupta0705 @anshgoyalevil @Mayaleeeee Please take a look at this PR. Thanks! 👋 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but looping @smoya @akshatnema for concrete approval & merge
@Shurtu-gal Are all test cases from https://github.com/asyncapi/website?tab=readme-ov-file#json-schema-definitions (decisions are diamonds) tested 🤔 ? |
I think so. Please let me know if sth is missing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🚀🌔
/ptal |
@derberg @magicmatatjahu @akshatnema @sambhavgupta0705 @anshgoyalevil @Mayaleeeee Please take a look at this PR. Thanks! 👋 |
/u |
Does anybody know how to make paths work in both windows and linux for jest tests? Context: https://github.com/asyncapi/website/actions/runs/10040697813/job/27747254031?pr=2493#step:10:9 |
/ptal |
@derberg @magicmatatjahu @akshatnema @sambhavgupta0705 @anshgoyalevil @Mayaleeeee Please take a look at this PR. Thanks! 👋 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Shurtu-gal 🚢
/rtm |
Description
Related issue(s)
Fixes #842